use 'lit' as the field name for literal values #16498
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
physical-expr
|
FYI @timsaucer |
timsaucer
approved these changes
Jun 22, 2025
|
Thanks folks. One more thought: I wonder if this improves performance by some small fraction given that I could see Anyway I pla to leave this open for review for another day or so then I'll merge it. |
alamb
reviewed
Jun 22, 2025
| @@ -66,8 +66,7 @@ impl Literal { | |||
| value: ScalarValue, | |||
| metadata: Option<FieldMetadata>, | |||
| ) -> Self { | |||
| let mut field = | |||
| Field::new(format!("{value}"), value.data_type(), value.is_null()); | |||
| let mut field = Field::new("lit".to_string(), value.data_type(), value.is_null()); | |||
There was a problem hiding this comment.
If we see string allocations on the hot path, perhaps we could do something like cache the FieldRef for each datatype or something 🤔
There was a problem hiding this comment.
(As a follow on PR, to be clera)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As per #16491 (comment) I think it's a bit strange that we try to create a field name from the repr of the value.
Consider this example: https://datafusion.apache.org/user-guide/sql/scalar_functions.html#id273
For cases of an array with hundreds of elements it will blow up and make a mess!
Could we use a fixed constant like
'lit'or'field'instead?The main issue I could see happening is name collisions, e.g.
select 1, 2, 3will cause an error which is unfortunate, not sure how to resolve that but also the current behavior isn't much better:FWIW Postgres seems to have the concept of an "un-named" column:
But I'm not sure we want to introduce an "unnamed" field.